Skip to content

Fix tab ID becoming 0 once the tab contains splits - #4208

Open
4RH1T3CT0R7 wants to merge 2 commits into
micro-editor:masterfrom
4RH1T3CT0R7:fix/4186
Open

Fix tab ID becoming 0 once the tab contains splits#4208
4RH1T3CT0R7 wants to merge 2 commits into
micro-editor:masterfrom
4RH1T3CT0R7:fix/4186

Conversation

@4RH1T3CT0R7

Copy link
Copy Markdown

Tab embeds its root views.Node, so tab:ID() in Lua ends up calling Node.ID(). That method returns 0 for any node that has children, and once a tab contains a split its root is no longer a leaf. A plugin that keys anything on the tab id loses track of the tab the moment the user splits it, which is what #4186 reports.

Tab now carries its own id, taken from the root node when the tab is created, and ID() returns that. For an unsplit tab the value is the same as before, it just no longer changes after a split. A small test in internal/action covers the split case and checks that two tabs get distinct ids.

Fixes #4186

`Tab` embeds its root `views.Node`, so `tab:ID()` from Lua resolved to
`Node.ID()`, which returns 0 for any node that has children. As soon as
a tab contained a split its root stopped being a leaf and the id was
gone.

Give `Tab` its own id, taken from the root node when the tab is created,
so it stays the same after splitting. An unsplit tab reports the same
value as before.
Comment thread internal/action/tab.go Outdated
Comment on lines +242 to +245
// id is this tab's unique id. It is taken from the root node when the
// tab is created and does not change afterwards, unlike the id of the
// embedded root node, which becomes 0 once the node has children.
id uint64

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure, if we really need to add a further id within the tab, which is already a view, instead of taking care of this:

// The id is unique for each leaf node and provides a way to keep track of a split
// The id cannot be 0
id uint64

// ID returns this node's id or 0 if it is not viewable
func (n *Node) ID() uint64 {
if n.IsLeaf() {
return n.id
}
return 0
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. 1af86dd drops the extra field and lets Node.ID() return n.id for containers too; a node that gets split keeps its id and hands it to the child taking its place, so GetNode() still resolves to the same leaf and only the 0 for non-leaf nodes goes away. One thing to be aware of: flatten() copies the last remaining child over the root, so once the pane a tab was created with is closed while another pane is left, the tab id becomes that pane's id (a root leaf has to carry the id of its only pane for GetNode() to find it, so I don't see a way around that within views); if that case matters I'd rather go back to the tab-level id.

`Node.ID()` returned 0 for any node with children. A tab embeds its root
node, so `tab:ID()` in Lua dropped to 0 as soon as the tab was split.

A node that gets split keeps its id and hands the same id to the child
that takes its place, so returning it from a container does not change
which leaf `GetNode()` finds. Drop the extra id on `Tab` from the
previous commit and test this in views instead.
@JoeKar

JoeKar commented Sep 7, 2026

Copy link
Copy Markdown
Member

@Neko-Box-Coder:
What do you think about it?

@Neko-Box-Coder

Neko-Box-Coder commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

With this change applied, when a split happens, wouldn't there be 2 nodes (tab parent node and the split node) with the same ID? If so, I don't think this should happen.

[Edit]:
If not, could you call Node.String() on the tab and paste the node trees before and after this merge in your initlua script? (You will need to run micro in debug mode with micro -debug.
If not I can do it when I am free, maybe later this week.

@4RH1T3CT0R7

Copy link
Copy Markdown
Author

Yes, after a split the container node and its first child hold the same id, but this change doesn't create that. hVSplit/vHSplit deliberately pass the old id down to the child that takes the split node's place (vn1 := NewNode(STVert, n.X, n.Y, n.W/2, n.H, n, n.id)), so the duplicate is already there on master; ID() was just hiding it by returning 0 once a node had children. Nothing resolves to the wrong node because GetNode() only matches leaves, and the scans in ResizeSplit, Unsplit and flatten compare siblings within a single children list.

Here is Node.String() on the tab, before a vsplit, after it, and after unsplitting again. I got these from a small Go test driving the same calls rather than from a plugin, and the output is byte-identical on master, since String() prints the id field instead of calling ID():

before vsplit:
{0 0 80 24} 1 0🍁

after vsplit:
-{0 0 80 24} 1 0
	|{0 0 40 24} 1 1🍁
	|{40 0 40 24} 2 1🍁

after unsplit:
{0 0 80 24} 1 0🍁

The only thing that differs is tab:ID() in the middle state: 0 before this patch, 1 after.

I did try the alternative of giving the container a fresh id so the ids stay unique, and it costs more than the bug is worth: the pane that was already there keeps splitID 1, GetNode(1) then returns nil because its leaf is now 3, and Tab.Resize() dereferences that nil, so VSplit/HSplit would have to return the reassigned id too and BufPane would have to update its own. I'd rather keep this patch and leave the tree's existing convention alone. If a tab reporting the same number as one of its panes is the real objection, that is already true before any split, since NewTabFromBuffer does e.SetID(t.ID()) — but I'm happy to go back to my first version instead, which gave Tab its own id copied from the root at creation and left views untouched.

@Neko-Box-Coder

Copy link
Copy Markdown
Contributor

@4RH1T3CT0R7

Thanks for your reply, I appreciate your graph and in-depth explanations.

The only thing that differs is tab:ID() in the middle state: 0 before this patch, 1 after.

Hm... while I understand what you are saying, I am kinda iffy about having nodes not having a unique ID tbh.

Previously if a lua end user wants to iterate/track buffer nodes, they can just store the IDs that are non-zero. With this change, however, they now would need to do an additional check to see if the node is leaf or not.

That being said, I am not aware of any plugins that rely on the node ID afaik.

I did try the alternative of giving the container a fresh id so the ids stay unique, and it costs more than the bug is worth: [...]

If you just need to have an unique Id for tracking tabs in lua, would storing the tab pointers themselves work? I think they are persistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tabs with splits return ID() = 0 (in Lua)

3 participants